bucket.Put: copy key before seek #637
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fuweid
force-pushed
the
copy-key-before-seek
branch
from
6c28521
to
1d491a7
Compare
ahrtr
reviewed
Dec 12, 2023
Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
force-pushed
the
copy-key-before-seek
branch
from
1d491a7
to
215e351
Compare
ahrtr
reviewed
Dec 13, 2023
tests/failpoint/db_failpoint_test.go
Outdated
Comment on lines
161
to
163
| // When application inserts key/value, that key can be changed by application | ||
| // during insertion. bbolt should copy key before seeking and updating. Otherwise, | ||
| // some branch node could point to freed page. |
There was a problem hiding this comment.
It doesn't necessarily cause elements to point freed pages. In this test case, if you change the key to a larger key e.g. 200, the case will pass.
Suggested change
| // When application inserts key/value, that key can be changed by application | |
| // during insertion. bbolt should copy key before seeking and updating. Otherwise, | |
| // some branch node could point to freed page. | |
| // When bbolt is processing a `Put` invocation, the key might be concurrently | |
| // updated by the application which calls the `Put` API (although it shouldn't). | |
| // It might lead to a situation that bbolt use an old key to find a proper | |
| // position to insert the key/value pair, but actually inserts a new key. | |
| // Eventually it might break the rule that all keys should be sorted. In a | |
| // worse case, it might cause page elements to point to already freed pages. |
There was a problem hiding this comment.
Thanks for the comment. Updated
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Application might change key value after seeking and before real put. This unexpected behaviour could corrupt database. When users file issue, maintainers doesn't know application behaviour. It could be caused by data race. This patch is to prevent such case and save maintainers' time. Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
force-pushed
the
copy-key-before-seek
branch
from
215e351
to
a05ec68
Compare
fuweid
added a commit
to fuweid/bbolt
that referenced
this pull request
Dec 17, 2023
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
added a commit
to fuweid/bbolt
that referenced
this pull request
Dec 17, 2023
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
added a commit
to fuweid/bbolt
that referenced
this pull request
Dec 18, 2023
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <fuweid89@gmail.com> (cherry picked from commit 62d8026) Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
added a commit
to fuweid/bbolt
that referenced
this pull request
Dec 18, 2023
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid
added a commit
to fuweid/bbolt
that referenced
this pull request
Dec 18, 2023
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
ishan16696
pushed a commit
to ishan16696/bbolt
that referenced
this pull request
Feb 25, 2024
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <fuweid89@gmail.com>
ishan16696
pushed a commit
to ishan16696/bbolt
that referenced
this pull request
Feb 25, 2024
It's follow-up of etcd-io#637. Signed-off-by: Wei Fu <fuweid89@gmail.com>
ishan16696
pushed a commit
to ishan16696/bbolt
that referenced
this pull request
Feb 25, 2024
- [bucket: allow to allocate key on stack in Put()](etcd-io#550) - [bucket.Put: copy key before seek](etcd-io#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
passiondev2024
added a commit
to passiondev2024/bbolt
that referenced
this pull request
Mar 20, 2024
- [bucket: allow to allocate key on stack in Put()](etcd-io/bbolt#550) - [bucket.Put: copy key before seek](etcd-io/bbolt#637) - [copy key before comparing during CreateBucket or CreateBucketIfNotExists](etcd-io/bbolt#641) Signed-off-by: Wei Fu <fuweid89@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Application might change key value after seeking and before real put.
This unexpected behaviour could corrupt database. When users file issue,
maintainers doesn't know application behaviour. It could be caused by
data race. This patch is to prevent such case and save maintainers' time.
REF: #72